You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The test creates a user context but doesn't explicitly clean it up after the test. Consider adding cleanup code to ensure the user context is properly disposed of after the test completes.
The test doesn't include any error handling for potential failures when creating contexts or switching between windows. Consider adding try-catch blocks or assertions to verify operations succeed.
The test should clean up the created user context after the test completes to prevent resource leaks. Add code to delete the user context in a try-finally block or at the end of the test.
Why: This suggestion addresses an important resource management issue. Not cleaning up user contexts could lead to resource leaks during test execution, especially when running multiple tests. The suggestion correctly identifies the need for proper cleanup.
Medium
Close browsing context
The test should close the newly created browsing context after it's no longer needed to prevent resource leaks. Add code to close the context after switching back to the original window.
driver.switchTo().window(windowHandle);
+context.close();
PartitionDescriptor browsingContextPartitionDescriptor =
new BrowsingContextPartitionDescriptor(windowHandle);
Apply this suggestion
Suggestion importance[1-10]: 8
__
Why: This suggestion correctly identifies a resource leak where the newly created browsing context is not properly closed after use. Failing to close contexts can lead to resource exhaustion when running multiple tests, making this an important fix for test stability.
Medium
Learned best practice
Add null validation for critical values returned from API calls to ensure clear failure messages
The test creates a user context and uses it without validating that it's not null. Since this is a critical part of the test, you should add a null check for the userContext value returned from browser.createUserContext() to ensure the test fails clearly if user context creation fails.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Motivation and Context
Types of changes
Checklist
PR Type
Tests
Description
Added a new test
canGetCookieInAUserContextto validate cookie handling in user contexts.Implemented user context creation and cookie operations using
BrowserandStoragemodules.Verified partition keys and cookie values for user-specific contexts.
Ensured proper cleanup and context switching in the test.
Changes walkthrough 📝
StorageCommandsTest.java
Added test for user context cookie handlingjava/test/org/openqa/selenium/bidi/storage/StorageCommandsTest.java
canGetCookieInAUserContext.Browsermodule to create user contexts.